Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

View / edit / reply to this conversation on ReviewNB

juanitorduz commented on 2025-10-08T11:36:22Z
----------------------------------------------------------------

Line #1.    seed: int = sum(map(ord, "pymc-marketing is more than just a marketing model"))

This can be on the "setting the notebook section"

View / edit / reply to this conversation on ReviewNB

juanitorduz commented on 2025-10-08T11:36:23Z
----------------------------------------------------------------

Provide some storytelling on what type of series and how many we plan to generate, and what's the objective.

cetagostini commented on 2025-10-08T16:42:36Z
----------------------------------------------------------------

Added above in the data generation process title.

View / edit / reply to this conversation on ReviewNB

juanitorduz commented on 2025-10-08T11:36:23Z
----------------------------------------------------------------

Line #1.    countries = ["Venezuela", "Colombia", "Ecuardo", "Panama"]

Typo "Ecuador"

cetagostini commented on 2025-10-08T16:43:14Z
----------------------------------------------------------------

Done!

View / edit / reply to this conversation on ReviewNB

juanitorduz commented on 2025-10-08T11:36:24Z
----------------------------------------------------------------

super title overlaps with some subtitles

cetagostini commented on 2025-10-08T16:46:15Z
----------------------------------------------------------------

Done!

View / edit / reply to this conversation on ReviewNB

juanitorduz commented on 2025-10-08T11:36:25Z
----------------------------------------------------------------

Line #19.                & (multi_country_df["product_type"] == product_type)

Tip: using strings with query can look more readable https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.query.html

View / edit / reply to this conversation on ReviewNB

juanitorduz commented on 2025-10-08T11:36:25Z
----------------------------------------------------------------

Same here for the titles

View / edit / reply to this conversation on ReviewNB

juanitorduz commented on 2025-10-08T11:36:26Z
----------------------------------------------------------------

Add a subsection "multidimensional model" just to separate sections better?

cetagostini commented on 2025-10-08T16:47:43Z
----------------------------------------------------------------

Done!

This notebook is fantastic @cetagostini ! 🚀 ! I left some minor comments.

Overall, I suggest we work out a better story telling and motivate at the beginning the plan and scope of the notebook (I think Claude Code can be suer helpful for this as the code is nicely written)

Added above in the data generation process title.

View entire conversation on ReviewNB

Done!

View entire conversation on ReviewNB

Done!

View entire conversation on ReviewNB

Done!

View entire conversation on ReviewNB

Maybe reference this in the https://www.pymc-marketing.io/en/latest/notebooks/mmm/mmm_components.html notebook. Maybe at the top

Two suggestions:

1. I wouldn't use the GAM language to describe this notebook. People would expect this to be a tutorial on adding non-parametric functions (especially splines). Wrapping effects up in a non-linear function like exp doesn't qualify it as a GAM as far as I can tell from consulting wikipedia and claude. Maybe some non-technical title like "Add anything to your MMM" would be appropriate because the goal is to provide a general recipe that lets power users do anything they cannot find pre-built in the docs.

2. We'd want some cleaner division of labour between the existing mmm_components notebook and this one. If I knew I wanted some special customization and saw both of these in the gallery, I wouldn't know which notebook to look at.

What do we think about retitling and refocusing the current components notebook to be "How to model seasonality and special events." @williambdean?

1. I wouldn't use the GAM language to describe this notebook. People would expect this to be a tutorial on adding non-parametric functions (especially splines). Wrapping effects up in a non-linear function like exp doesn't qualify it as a GAM as far as I can tell from consulting wikipedia and claude. Maybe some non-technical title like "Add anything to your MMM" would be appropriate because the goal is to provide a general recipe that lets power users do anything they cannot find pre-built in the docs.

@daniel-saunders-phil agree with second point, regarding the first not fully sure. Because, as I see it MMM can indeed be viewed as a subclass or applied form of a GAM. My view is this:

1. Every MMM is a GAM > ❌ Not necessarily (if fully parametric)
2. A flexible MMM can be implemented as a GAM > ✅ Absolutely — This is the advantageous point of our class.

If you want spine lines, thats definitely possible to implement. I could create customization to implement both saturation and adstock non-parametrically, I'm not saying I'll do it, but if someone does it (and our model can implemented) then why this could not be consider a GAM?

Probably adding this as comment could be enough, no? Will show the point and close the main idea.
ps: This was my idea to bring the notebook to existence, nevertheless, I can be wrong! jeje

@juanitorduz render checks done. But the error in the test looks unrelated.

@carlosagostini According to Copilot:

The job failed because the GitHub Actions runner was terminated before all tests finished, as indicated by the error: "The runner has received a shutdown signal... The operation was canceled." This is not a code failure, but an infrastructure/runtime issue. The logs also show warnings about leaked semaphore, semlock, and folder objects, which are common when jobs are forcefully stopped.

Solution:

1. Re-run the job: This type of failure is usually transient. Use the "Re-run jobs" button in GitHub Actions to try again.
2. If you frequently see runner shutdowns:
   Check for timeouts in your workflow definition (.github/workflows/test_notebook.yml). Increase timeout limits if necessary (e.g., add or update timeout-minutes: for your steps/jobs).

Ensure the notebooks (like mmm_components.ipynb and mmm_time_slice_cross_validation.ipynb) do not run overly long or consume excessive resources. Optimize their code if possible.

Can you try solution 2, adding the timeout-minutes: parameter ?

Maybe reference this in the https://www.pymc-marketing.io/en/latest/notebooks/mmm/mmm_components.html notebook. Maybe at the top

Was this reference added?

@cetagostini , Copilot suggests we modify the runner.py script
If you use joblib.Parallel, modify usage as follows:

from joblib import Parallel, delayed

results = Parallel(n_jobs=4)(delayed(my_func)(i) for i in range(10))
del results
import gc
gc.collect()

maybe worth a try?

but if someone does it (and our model can implemented) then why this could not be consider a GAM?

It's just misleading to label the notebook as being about GAMs if it only presents linear and non-linear regressions. As a reader, I would expect at least one GAM in the GAM notebook.

but if someone does it (and our model can implemented) then why this could not be consider a GAM?

It's just misleading to label the notebook as being about GAMs if it only presents linear and non-linear regressions. As a reader, I would expect at least one GAM in the GAM notebook.

This is a fair point @daniel-saunders-phil . I also imagine we are targeting MMM practitioners, so instead of GAMS we could have something more explicit like MMM with custom additive components (or something along those lines but more sexy XD)

@cetagostini maybe this can help #1986

@daniel-saunders-phil really agree with you, we should not say GAMs without one!

I build a non parametric form for Saturation and Adstock, as consequence the model with trend, season, and non parametric trasnformations can be considered a GAM. So, this precisely show the point about how to jump from linear models, traditional MMMs, semi-parametric GAM models to fully MMM-GAMs like models.

What do we think? @juanitorduz

How do you leave in-text comments a PR on the notebook reader website thing? I can see the new GAM content but it won't let me comment on it.

regardless, it looked good to me. Only a typo "our Generative Additive Media Mix Model is alive and sampling" -> "our Generalized Additive Media Mix Model is alive and sampling"

Could it be that the new notebooks do some really heavy computations? I would not know where

I would be careful here.

A user may be tempted to think that they can call optimize on top of whatever custom MMM they define, and this is not the case.

I would be careful here.

A user may be tempted to think that they can call optimize on top of whatever custom MMM they define, and this is not the case.

What do you mean? If they follow the wrapper, they could do. What are cases where that's not true? 🤔 @PabloRoque

@juanitorduz found issue. The approximate fit method (advi) for large models was taking 2 minutes, I take it out and keep example but only with the simple model.